-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BiG-CZ: WDC / CUAHSI Details View #2259
Conversation
<p> | ||
<!-- TODO Make This Work --> | ||
<a class="zoom" href="#"> | ||
<i class="fa fa-search-plus"></i> Zoom to extent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that all WDC results were point-based. If that's the case, it may not make sense to have a "zoom to extent" feature.
Also, that description comes in from the API as plain text. We don't know it has links. We'll have to use a regex or something to transform them into links. |
Good point. It may make sense to let it be as is for now. |
The difference here is that we're not providing the content for the popup. They're indicating that we'd have to parse for things that appear to be links and convert them from plain text. |
I don't think we have to convert them, but even to support people copy-pasting a plain text link, they need to be able to click on the popup. |
Tooltip stuff sounds like a good follow-up card. |
Just to clarify, the citation values coming back from the server sometimes have placeholder values, like in the PR screenshot? |
Yeah, like this:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Nice job splitting up the commits into related chunks of work.
from rest_framework.serializers import CharField, DateTimeField | ||
from rest_framework.serializers import (CharField, | ||
DateTimeField, | ||
ListField |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, but consider adding a trailing comma here.
These will allow us to do really catalog specific things in each detail view.
Previously we were joining list values into strings before sending them to the client. This dictated client presentation, and made it difficult to treat the data differently. To allow clients to decide how best to show these lists, they are now sent back as lists. The client template is updated to maintain current behavior.
These values will be used in the front-end.
* Title now features location, which is extracted from the id * Source has service_org and service_title, with the description inside a popup * Instead of the calendar icon, we now use the text "Data collected on" or "Data collected between" based on if the end date is different from the beginning date * Sample mediums are listed * Source Data button is shown if details_url exists * Web Services button is shown pointing to the service url * Last collected value is shown in relative time, using a new toTimeAgo filter * A table of concept keywords is added - This currently only features the keyword name - Values and units will be pulled in the future, see #2238 * There is comment placeholder for adding charts, coming in #2238 * Citation in the bottom * Some style edits to make it all fit The popover() and bootstrapTable() functions will only execute if the template has any corresponding data-toggle elements. Thus, they noop for CINERGI and HydroShare.
Previously, with 'focus', the popover would disappear after a click. With 'click', it stays around until explicitly closed, either by the close button within the popover, or by re-clicking the info icon that opened it. This allows the user to select text in the popover, and in the case of URLs, paste them into new tabs to visit the relevant sites.
73f0eff
to
86e33cb
Compare
Thanks for the review! Squashed all the fixups, added that last trailing comma, will merge now. |
This all looks great. Did you all figure out that the text after "Citation:" comes directly from the "citation" field in the response from a GetServices request, which should be cached for all the services? See the Sample_WDC_Site_Detail_BiGCZPortal_SearchResult Google Doc that I set up as a mockup and map to web service response fields. |
Yes, those values are coming from the (soon to be cached, see #2260) Services request, as specified in the Google Doc. Thanks for setting that up, it made the implementation considerably more straightforward. |
Overview
Adds specific items to CUAHSI's details view, based on comments in #2234 (comment) and this Google doc. Important to note: this does not fetch the detail information for a given item. That will be done in #2238, which depends on #2243.
Also tagging @aufdenkampe for visual review.
Connects #2234
Demo
Notes
Some fields have placeholder values, such as the citation in the above screenshot. These placeholders seem arbitrary, and to convert them all into real values may be a long-tailed effort. Currently they are left as is.
Testing Instructions
bundle